[Test] Refine backward compat test on API version change #5276
Conversation
Signed-off-by: Aylei <[email protected]>
aylei
changed the title
|
/quicktest-core |
Signed-off-by: Aylei <[email protected]>
|
/quicktest-core |
Signed-off-by: Aylei <[email protected]>
|
/quicktest-core |
Signed-off-by: Aylei <[email protected]>
|
/quicktest-core https://buildkite.com/skypilot-1/quicktest-core/builds/288#01964875-d63a-4d71-a298-aaad84dc7f84 can be successfully skipped when version bump |
Signed-off-by: Aylei <[email protected]>
|
/quicktest-core https://buildkite.com/skypilot-1/quicktest-core/builds/289#019648a5-f70b-44d9-b3b3-54db99196bc8 Compatibility is checked when API version is not changed |
| # Check API version compatibility | ||
| # If API version is bumped, the in-compatibility is expected | ||
| # and we just skip the test. | ||
| skip_if = ( |
There was a problem hiding this comment.
I was wondering if we actually need to introduce the skip_if parameter. It's not a straightforward boolean parameter, and you'd need to read the source code to understand how skip_if behaves.
An alternative could be
# pesudo code
BASE_SERVER_VERSION = subprocess.popen('self.ACTIVATE_BASE && python -c "print(sky.__server_version__)"')
CURRENT_CLIENT_VERSION=subprocess.popen('{self.ACTIVATE_CURRENT} && python -c "print(sky.__client_version__)"'')
if mismatch(BASE_SERVER_VERSION, CURRENT_CLIENT_VERSION):
pytest.skip()
...
self.run_compatibility_test(cluster_name,
commands,
teardown)There was a problem hiding this comment.
How about make skip_if a callable that returns boolean? I was intended to use a sky command to check compatibility so that the implementation details of compatibility can be kept in a blackbox for smoke test
There was a problem hiding this comment.
Yeah, that sounds better that way.
Signed-off-by: Aylei <[email protected]>
Follow up: #5255
close: #5277
Currently if API version is bumped, there would be an test failure which is expected: https://buildkite.com/skypilot-1/quicktest-core/builds/282#0196423d-94af-4954-b514-2bfb3d4f925f
This PR change the status to success in this case to avoid confusions.
Tested (run the relevant ones):
bash format.sh/smoke-test(CI) orpytest tests/test_smoke.py(local)/smoke-test -k test_name(CI) orpytest tests/test_smoke.py::test_name(local)/quicktest-core(CI) orpytest tests/smoke_tests/test_backward_compat.py(local)